Add compile-time fault-injection infrastructure for stack-walker recovery paths#661
Add compile-time fault-injection infrastructure for stack-walker recovery paths#661zhengyu123 wants to merge 10 commits into
Conversation
CI Test ResultsRun: #29506661506 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-07-16 14:46:07 UTC |
Benchmark Results (commit d85f055)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/124877517 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10232 ms (21 iters) | ✅ 10166 ms (21 iters) | ≈ -0.6% (±11.1%) | — / — |
| akka-uct | 25 | ✅ 8907 ms (24 iters) | ✅ 8929 ms (24 iters) | ≈ +0.2% (±9.6%) | — / — |
| finagle-chirper | 21 | ✅ 5972 ms (33 iters) | ✅ 5984 ms (33 iters) | ≈ +0.2% (±25.3%) | |
| finagle-chirper | 25 | ✅ 5441 ms (36 iters) | ✅ 5530 ms (36 iters) | ≈ +1.6% (±25%) | |
| fj-kmeans | 21 | ✅ 2734 ms (69 iters) | ✅ 2707 ms (70 iters) | ≈ -1% (±2.7%) | — / — |
| fj-kmeans | 25 | ✅ 2822 ms (66 iters) | ✅ 2833 ms (66 iters) | ≈ +0.4% (±2.6%) | — / — |
| future-genetic | 21 | ✅ 2088 ms (89 iters) | ✅ 2087 ms (89 iters) | ≈ -0% (±2.7%) | — / — |
| future-genetic | 25 | ✅ 2025 ms (92 iters) | ✅ 2137 ms (87 iters) | 🔴 +5.5% | — / — |
| naive-bayes | 21 | ✅ 1286 ms (134 iters) | ✅ 1344 ms (128 iters) | ≈ +4.5% (±33%) | — / — |
| naive-bayes | 25 | ✅ 1010 ms (170 iters) | ✅ 1027 ms (166 iters) | ≈ +1.7% (±32.1%) | — / — |
| reactors | 21 | ✅ 16543 ms (15 iters) | ✅ 16267 ms (15 iters) | ≈ -1.7% (±8%) | — / — |
| reactors | 25 | ✅ 18414 ms (15 iters) | ✅ 18374 ms (15 iters) | ≈ -0.2% (±4.2%) | — / — |
Internal counter details (ddprof)
ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
| Benchmark | JDK | Dropped rec | Dropped jvmti | Dropped trace | Skipped WC | AGCT fail | Unwind fail |
|---|---|---|---|---|---|---|---|
| akka-uct | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / 3 | 2017 / 2006 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 3 | 2142 / 2423 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 5 / 2 | 8761 / 8937 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 4 | 8332 / 8785 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 2 / 2 | 1285 / 1288 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 2 / 3 | 1273 / 1302 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / 1 | 3049 / 3118 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 1 | 2892 / 2926 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 3 / 6 | 3506 / 3515 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 4 / 2 | 3476 / 3495 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | 2 / ✅ | 1758 / 1644 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / 2 | 1913 / 1843 | ✅ / ✅ | ✅ / ✅ |
Reliability & Chaos Results✅ All reliability & chaos checks passed Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/124877424 |
There was a problem hiding this comment.
Pull request overview
This PR introduces an opt-in, compile-time fault-injection layer (__FAULT_INJECTION__) to deliberately corrupt pointer/value reads at real profiler memory-access sites, so existing recovery paths (SafeAccess safefetch and walkVM setjmp/longjmp) are continuously exercised under test without affecting production builds.
Changes:
- Added new fault-injection infrastructure (
faultInjection.h/.cpp) with tiered probabilities and a per-thread PRNG seed/state. - Wrapped key memory-access sites (stack walker, HotSpot walkVM, VMStructs) with injection macros and added counters to observe injection/recovery activity.
- Wired
-PenableFaultInjectioninto native build configurations and gtest builds; added a dedicated gtest suite for enabled/disabled behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ddprof-lib/src/main/cpp/faultInjection.h | New public fault-injection API/macros gated by __FAULT_INJECTION__. |
| ddprof-lib/src/main/cpp/faultInjection.cpp | New implementation: PRNG, guard-region setup, poison address generation, injected int/long values. |
| ddprof-lib/src/main/cpp/threadLocalData.h | Adds per-thread xorshift64 RNG state and accessors under __FAULT_INJECTION__. |
| ddprof-lib/src/main/cpp/stackWalker.cpp | Wraps SafeAccess dereference sites with fault-injection macros. |
| ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp | Wraps selected raw walkVM dereferences; increments longjmp recovery counter. |
| ddprof-lib/src/main/cpp/hotspot/vmStructs.h | Injects faults in VMStructs::at() return pointers (rare tier). |
| ddprof-lib/src/main/cpp/safeAccess.cpp | Adds counter increments on safefetch/safecopy recovery paths. |
| ddprof-lib/src/main/cpp/profiler.cpp | Initializes fault-injection guard region during signal-handler setup (when enabled). |
| ddprof-lib/src/main/cpp/counters.h | Adds new counters for safefetch/safecopy failures, injected faults, and walkVM longjmp recovery. |
| ddprof-lib/src/test/cpp/faultInjection_ut.cpp | New unit tests for disabled identity behavior and enabled fault/recovery behavior. |
| build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt | Adds -D__FAULT_INJECTION__ to debug/release when -PenableFaultInjection is set. |
| build-logic/conventions/src/main/kotlin/com/datadoghq/native/gtest/GtestTaskBuilder.kt | Mirrors the fault-injection define into gtest compilation under -PenableFaultInjection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bool SafeAccess::handle_safefetch(int sig, void* context) { | ||
| ucontext_t* uc = (ucontext_t*)context; | ||
| uintptr_t pc = uc->current_pc; | ||
| if ((sig == SIGSEGV || sig == SIGBUS) && uc != nullptr) { | ||
| if (pc == (uintptr_t)safefetch32_impl) { |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Benchmark Results (commit f1c6dc1)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/124997579 Commit: ✅ Within expected boundariesNo significant runtime deltas (all within run-to-run noise) and no internal-counter outliers. Runtime details (per benchmark × JDK)
Internal counter details (ddprof)ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
|
Benchmark Results (commit b88f34e)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125033402 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10294 ms (21 iters) | ✅ 10207 ms (21 iters) | ≈ -0.8% (±11.4%) | — / — |
| akka-uct | 25 | ✅ 8806 ms (24 iters) | ✅ 8847 ms (24 iters) | ≈ +0.5% (±10.2%) | — / — |
| finagle-chirper | 21 | ✅ 5993 ms (33 iters) | ✅ 5964 ms (33 iters) | ≈ -0.5% (±25.4%) | |
| finagle-chirper | 25 | ✅ 5507 ms (36 iters) | ✅ 5410 ms (36 iters) | ≈ -1.8% (±23.7%) | |
| fj-kmeans | 21 | ✅ 2746 ms (68 iters) | ✅ 2751 ms (68 iters) | ≈ +0.2% (±2.7%) | — / — |
| fj-kmeans | 25 | ✅ 2774 ms (68 iters) | ✅ 2864 ms (66 iters) | 🔴 +3.2% | — / — |
| future-genetic | 21 | ✅ 2097 ms (89 iters) | ✅ 2095 ms (89 iters) | ≈ -0.1% (±2.6%) | — / — |
| future-genetic | 25 | ✅ 2079 ms (89 iters) | ✅ 2142 ms (87 iters) | 🔴 +3% | — / — |
| naive-bayes | 21 | ✅ 1267 ms (135 iters) | ✅ 1274 ms (134 iters) | ≈ +0.6% (±32.7%) | — / — |
| naive-bayes | 25 | ✅ 1012 ms (169 iters) | ✅ 1007 ms (170 iters) | ≈ -0.5% (±31.2%) | — / — |
| reactors | 21 | ✅ 16315 ms (15 iters) | ✅ 16117 ms (15 iters) | ≈ -1.2% (±7.6%) | — / — |
| reactors | 25 | ✅ 18032 ms (15 iters) | ✅ 18381 ms (15 iters) | ≈ +1.9% (±5.9%) | — / — |
Internal counter details (ddprof)
ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
| Benchmark | JDK | Dropped rec | Dropped jvmti | Dropped trace | Skipped WC | AGCT fail | Unwind fail |
|---|---|---|---|---|---|---|---|
| akka-uct | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 1 | 1961 / 1910 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 2 / 2 | 2182 / 2135 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 4 / 2 | 8807 / 8361 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | 2 / ✅ | 8192 / 8209 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 3 / ✅ | 1255 / 1265 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 4 / 2 | 1301 / 1285 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 3 | 2941 / 2957 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | 2 / 2 | 2834 / 2878 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 5 / 5 | 3488 / 3503 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 2 | 1714 / 1536 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / 1 | 1876 / 1844 | ✅ / ✅ | ✅ / ✅ |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…r into zgu/fault-injection
|
What does this PR do?:
Adds a compile-time fault-injection layer that deliberately corrupts pointers at the profiler's real memory-access sites so the existing recovery paths (SafeAccess safefetch and walkVM's setjmp/longjmp) are exercised continuously under test. Entirely gated behind FAULT_INJECTION: when the flag is absent every construct is a strict identity/no-op with zero runtime cost, so production release/debug/asan/tsan builds are unaffected.
To build fault-injected binaries, add
-PenableFaultInjectionflag to the command line, e.g.Motivation:
Additional Notes:
What's included
faultInjection.h — three probability tiers as integer xorshift thresholds (RARE 0.01%, UNLIKELY 0.1%, LIKELY 1%) and the INJECT_FAULT_{ADDRESS,INT,LONG}_{RARE,UNLIKELY,LIKELY} macros. When the flag is off, each macro is a strict identity ((ptr)/(v)).
faultInjection.cpp — PRNG, PROT_NONE guard-region setup, and poisonAddress() (mix of a mmap'd guard-page pointer and random non-canonical addresses).
Per-thread PRNG
threadLocalData.h — per-thread xorshift64 state in ProfiledThread (async-signal-safe), with a global atomic fallback.
Injection sites (wrap the dereferenced pointer)
stackWalker.cpp — walkFP / walkDwarf SafeAccess loads (LIKELY) → safefetch recovery.
hotspot/hotspotSupport.cpp — walkVM raw derefs (UNLIKELY) → setjmp/longjmp recovery.
hotspot/vmStructs.h — VMStructs::at() (RARE).
profiler.cpp — calls faultinj::init() at signal-handler setup.
Counters (counters.h, live under -DCOUNTERS)
safecopy_failed, safefetch_failed, faults_injected, walkvm_longjmp_recovered — instrument the recovery mechanisms so injection activity and each recovery route are observable.
Build wiring
-PenableFaultInjectionappends-D__FAULT_INJECTION__to the standard release/debug library builds (so buildDebug/buildRelease produce a fault-injected libjavaProfiler.so) and to the gtest build. Deliberately not applied to asan/tsan/fuzzer, which install their own SIGSEGV interception.faultInjection_ut.cpp — disabled-path identity checks (default build) plus enabled-path tier-rate bands and SafeAccess/walkVM recovery tests (-PenableFaultInjection).
How to test the change?:
./gradlew :ddprof-lib:gtestDebug_faultInjection_ut -PenableFaultInjection— 6 enabled cases pass../gradlew :ddprof-lib:gtestDebug_faultInjection_ut (default)— 2 identity cases pass../gradlew -PenableFaultInjection :ddprof-lib:assembleDebug— verified faultinj::* symbols present (.symtab; hidden visibility, not exported).For Datadog employees:
credentials of any kind, I've requested a security review (run the
dd:platform-security-reviewskill, or file a request via the PSEC review form).
bewairealso runs automatically on every PR.Unsure? Have a question? Request a review!